Skip to content

Conversation

@politikl
Copy link

x.py: Improve bootstrap import error handling and warning clarity

This change improves developer experience when invoking x.py in misconfigured environments.

Summary

This pull request improves the startup behavior of x.py by providing clearer
and more user-friendly error handling, without changing any functional behavior
or Python version requirements.

Details

  • Improved error message:
    If the src/bootstrap directory is missing (for example, when running
    x.py from an unexpected location), the script now exits with a clear,
    actionable message instead of failing with a less informative traceback.

  • Cleaner warning output:
    The stacklevel for Python version warnings has been changed from 1 to 2,
    ensuring that the warning message correctly points to the user's invocation
    rather than an internal line within the script.

  • Graceful import handling:
    The import bootstrap statement is now wrapped in a try/except block to
    provide a friendlier message if the module cannot be imported.

Rationale

These small improvements make it easier for contributors to understand and fix
environment misconfigurations during Rust’s bootstrap process, especially for
new contributors building from source. The change does not alter any
existing functionality, Python version logic, or fallback behavior.

Testing

  • Verified locally that:
    • ./x.py check and ./x.py build continue to work as expected.
    • A missing src/bootstrap directory produces the new clear error message.
  • No regressions observed on supported Python versions.

Notes

  • No functional or behavioral changes.
  • Maintains Python 2 fallback logic for compatibility with older systems.
  • Safe, self-contained, and low-risk improvement.

Example output with missing directory:

Error: expected bootstrap directory not found at /path/to/rust/src/bootstrap

Maintainers: This is a small developer-experience improvement and should be safe to merge without affecting CI or bootstrap logic.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Oct 26, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 26, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

Improve bootstrap import error handling and warning clarity
@rustbot
Copy link
Collaborator

rustbot commented Oct 26, 2025

⚠️ Warning ⚠️

  • The following commits have merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

    You can start a rebase with the following commands:

    $ # rebase
    $ git pull --rebase https://github.com/rust-lang/rust.git master
    $ git push --force-with-lease
    

@rustbot rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 26, 2025
@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member

bjorn3 commented Oct 26, 2025

If the src/bootstrap directory is missing (for example, when running
x.py from an unexpected location), the script now exits with a clear,
actionable message instead of failing with a less informative traceback.

If you run x.py from a different location, it will still be able to locate src/bootstrap. x.py looks relative to it's own location.

The import bootstrap statement is now wrapped in a try/except block to
provide a friendlier message if the module cannot be imported.

I don't see the point in this. This error can't happen unless you changed any of the python code. And if you do, the builtin python exception message is fine.

The stacklevel for Python version warnings has been changed from 1 to 2,
ensuring that the warning message correctly points to the user's invocation
rather than an internal line within the script.

What user invocation? Can you show the warning that would be printed before and after this PR?

@politikl
Copy link
Author

Thanks for the feedback!

If you run x.py from a different location, it will still be able to locate src/bootstrap.

You're absolutely right — I verified that x.py already resolves src/bootstrap relative to its own location, so this extra existence check doesn't add value. I can remove that part.

The import bootstrap statement is now wrapped in a try/except block to provide a friendlier message if the module cannot be imported.

Good point. I agree that this situation would only happen if the bootstrap code itself is broken, in which case Python’s native traceback is indeed clearer. I’ll revert that change as well.

The stacklevel for Python version warnings has been changed from 1 to 2... What user invocation?

Sure! Here’s an example of the difference.

Before (stacklevel=1):

/path/to/rust/x.py:53: UserWarning: Using python 3.5 but >= 3.6 is recommended.
  warnings.warn(msg, stacklevel=1)

After (stacklevel=2):

UserWarning: Using python 3.5 but >= 3.6 is recommended.

The goal was to remove the internal file reference (x.py line number) so that the warning appears cleaner to users running the script. That said, this is minor — I can drop this change too if it’s considered unnecessary.


If maintainers prefer keeping x.py minimal, I’m happy to simplify this PR to just the improved warning text (without changing stacklevel or import behavior).

@Kivooeo
Copy link
Member

Kivooeo commented Oct 26, 2025

I don't feel like we want to land this, the current error handling is sufficient and these changes don't address any real problem as far as I see

@rust-log-analyzer

This comment has been minimized.

@politikl
Copy link
Author

Thanks for the feedback! I understand this doesn't address a critical need.
I'll close this PR. Appreciate you taking the time to review!

@politikl politikl closed this Oct 26, 2025
@rustbot rustbot removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 26, 2025
@Kivooeo
Copy link
Member

Kivooeo commented Oct 26, 2025

In any case, I see this is your pull request, thanks for trying to contribute!

A good place to start is by looking through issues with labels like C-cleanup, E-needs-test, E-needs-bisection, and A-diagnostics, these are often great first issues for new contributors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has-merge-commits PR has merge commits, merge with caution. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants